refactor(uts): make :uts a shared test-infra module, move UTS suites to owning modules; objects spec-conformance - #1229
Merged
sacOO7 merged 10 commits intoAug 27, 2026
Conversation
…uite into :liveobjects
Three related changes from the cross-SDK objects audit follow-up:
1. Shared UTS test infra (mock transport, FakeClock, SandboxApp, proxy control)
moves from :uts's src/test to its src/testFixtures variant, so other modules
can consume it via testFixtures(project(":uts")). Acyclicity invariant
documented: :liveobjects test -> :uts testFixtures -> :java, with :uts test ->
:liveobjects kept runtime-only.
2. The objects UTS unit suite moves out of :uts into the :liveobjects module's own
test source set (package io.ably.lib.liveobjects.uts.unit) so the internal-graph
specs can reach `internal` members directly. Coverage expands: adds
InternalLiveCounter/Map, ObjectId, ObjectsPool and ParentReferences suites.
runLiveObjectsUnitTests now covers both .unit.* and .uts.unit.*.
3. Spec-conformance in production source:
- op-path applyObject/applyOperation now returns the ObjectUpdate instead of a
Boolean (RTLC9g/RTLM7f); the RTO9a2a4 on-ack gate uses !update.noOp.
- root object is excluded from GC (RTO10c1b1) and rejects tombstone attempts
(RTLO4e10); both covered by new tests.
Deviations recorded in liveobjects/.../uts/deviations.md. Unit suites and the CI
static-analysis gate are green.
|
Warning Review limit reachedNext included review available in 34 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (86)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The channel-state handler routes the ATTACHED transition to the sync lifecycle (RTO4) and all other states per RTO27, so add a method KDoc tagging both spec points, inline RTO27a/RTO27a1/RTO27a2 tags on the DETACHED/FAILED clear (with SUSPENDED excluded and retained per RTO27b), and an RTO27b tag on the else branch. Comment/doc only; no behaviour change. Mirrors the ably-js actOnChannelState tags.
…nd add their UTS unit tests - RTO23c1: a get() parked waiting for objects sync now fails when the channel enters DETACHED/SUSPENDED/FAILED — ensureSynced routes through the shared pendingSyncWaiters, each waiter carrying a caller-specific failure description (the object could not be retrieved vs RTO20e1's operation could not be applied locally), built into the 92008/400/cause error at the failure site. - RTO5a6: a malformed OBJECT_SYNC channelSerial (no ':' separator) is normalized to null so it takes the same branch as an absent serial (RTO5a5), with a warning logged. - Add the five UTS unit tests derived from the new spec cases (3x RTO23c1 per channel state, RTO5a5, RTO5a6). - Annotate the implementation sites of the newly specified points (RTO20d4, RTLC14c, RTLM22c). Spec changes: ably/specification#514 Companion ably-js fix: ably/ably-js#2284
Port the seven no-op-package UTS cases: RTLC14c/RTLM22c (zero-delta/empty diffs are no-op updates, never delivered), RTO20d4 (empty synthetic list skips the RTO20e sync wait), the RTLO5 tombstone-of-zero/empty-object cases and the RTLO4b4c3c zero-valued-counter teardown case (covering BaseRealtimeLiveObject.tombstone()'s NoOp-synthesis branch for the first time), and RTO4b2a (reset of an already-empty root emits no update; verified with a second-pool liveness control via a backward-compatible optional target parameter on the ObjectsPoolTest processAttached helper). Production already conforms at every site; test-only change. Spec changes: ably/specification#515 Companion ably-js fix: ably/ably-js#2288
…(OMP4a1) Message-size accounting matches Ably's published per-field rule: every plain string field and map key is measured as its UTF-8 byte length, while extras keeps the documented "string length of its JSON representation" (UTF-16 code units). Sites changed: - WireObjectMessage.kt: WireObjectsMap.size (OMP4a1) key measurement it.key.length -> it.key.byteSize, so map-state entry keys now match the MapCreate/MapSet/MapRemove operation keys; fixed a duplicated-// comment typo; corrected the WireObjectData json branch comment from OD3e to OD3g; extras keeps gson.toJson(it).length (UTF-16) now with an explanatory comment. Tests: +1 non-ASCII test testObjectMapStateEntryKeyUnicodeSizeIsUtf8 (OMP4a1). Spec: ably/specification#516
…to their owning modules
:uts's shared test infrastructure is promoted from the java-test-fixtures
variant to a normal main source set, and the spec-derived UTS suites move
to the modules that own the code they test:
- Infra: uts/src/testFixtures -> uts/src/main (16 pure renames, packages
io.ably.lib.uts.infra.* unchanged). :uts is now java-library + kotlin.jvm
and api-exports the UTS test toolkit (junit-bom/jupiter/params,
kotlin-test-junit5, coroutines) so consumers need only
testImplementation(project(":uts")). ktor stays implementation.
- Realtime tiers -> :java at lib/src/test/kotlin (packages unchanged; new
:java:runUtsUnitTests / :java:runUtsIntegrationTests Jupiter tasks; the
64 legacy JUnit4 tests and suite tasks are untouched; kotlin-stdlib is
kept out of the published artifact - POM/jar verified clean).
- Objects integration/proxy tiers -> :liveobjects at .../uts/{integration,
proxy}, joining the existing uts/unit; :liveobjects adopts the JUnit
Platform (vintage engine runs its own legacy JUnit4 tests).
- :uts keeps three permanent, deep tier smoke tests (unit/integration/
proxy) modeled on ably-cocoa#2223 - infra acceptance + the teaching
examples uts/README.md now walks through.
- uts-to-kotlin skill: mapping simplified to one repo-root-relative path
per tier; resolver emits the owning module; docs re-pointed.
- CI: check.yml and integration-test.yml re-pointed so every moved suite
keeps exactly one CI home (no silent-green).
Verified: 533 tests green across all tiers (98 java unit, 6+2 UTS unit,
389 objects unit, 5+4+29 integration/proxy); @uts test-id parity proven
(27 ids, zero loss); checkstyle/codenarc clean.
…iescence FakeClock Fixes the CI-red UnitInfraSmokeTest race and lands the review/spec-alignment round on the shared UTS infra: - Root cause of the CI flake: FakeClock.waitOn performs a real timed wait, so the disconnected-retry fires on wall-clock regardless of advance() — the "no attempt before advance" assertion was unassertable. The smoke test now owns attempt #2 via the buffered awaitConnectionAttempt() (32/32 green incl. CPU-saturation runs) and README §6.4/§9 teach the true semantics. - FakeClock: advance() now runs due work to quiescence (cascades and timers created mid-advance fire within the same advance — the spec's Fake-time semantics Guarantee); timers/pending hardened against SDK-thread races. The waitOn advisory seam is unchanged. New cascade smoke test covers it. - Mock contract fixes from review triage (verified against the UTS docs): transport cancel() now delivers listener.onClose; respondWith honors the headers param and JSON-serializes non-String bodies; SandboxApp checks HTTP status before parsing; delivery executor shutdown; @volatile channel fields; await helpers unregister listeners on success; AtomicReference for the cross-thread query-params capture. - Docs: uts/README rewritten claims verified against sources; stale "reflection" wording fixed in the skill's objects-mapping notes.
- DefaultPendingConnection: submit -> execute so a delivery exception reaches the thread's uncaught handler instead of a discarded Future. - DefaultPendingRequest: derive the body content-type from a case-insensitive Content-Type header lookup (default application/json) so caller-supplied headers are honored without conflicting metadata. - MockWebSocket: null activeListener on client-initiated close, matching every other close path; post-close sends now fail fast. - SandboxApp: delete() rethrows CancellationException (cooperative cancellation preserved); other errors remain best-effort-ignored per the documented teardown contract.
…ribing The test seeded a second path (alias) via send_to_client and subscribed immediately; the SDK applies inbound messages asynchronously, so on a slow runner the seed's MAP_SET dispatch raced the subscription and the alias listener saw two events (seed + increment) instead of one. Await the seed's observable effect first — the same hardening the depth tests in this file already use, per the documented async-delivery caveat in the skill's objects-mapping notes.
…nd-suite-redistribution refactor(uts): make :uts a shared test-infra module and move UTS suites to their owning modules
sacOO7
changed the title
sacOO7
marked this pull request as ready for review
August 27, 2026 11:30
sacOO7
merged commit Aug 27, 2026
9afa940
into
fix/liveobjects-objects-audit-op-handling
15 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to the cross-SDK objects audit, in two stages that landed on this branch (the second — #1231 — was reviewed separately and merged here). Net effect, four related changes:
:utsbecomes a shared test-infra module — the UTS infra (mock WebSocket/HTTP transports,FakeClock,SandboxApp, proxy control) is now:uts's main source set, and the moduleapi-exports the UTS test-writing toolkit (JUnit 5 BOM/aggregator/params, thekotlin-testJupiter binding, coroutines). Consumers need exactly one line:testImplementation(project(":uts")). (An intermediatetestFixturesstage existed during development and was superseded on this same branch — the net diff contains nojava-test-fixtures.):liveobjects(…/uts/{unit,integration,proxy}, with the unit tier expanded: newInternalLiveCounter,InternalLiveMap,ObjectId,ObjectsPool,ParentReferencessuites); realtime unit/integration/proxy →:javaatlib/src/test/kotlin/io/ably/lib/uts/…(new Jupiter-only:java:runUtsUnitTests/:java:runUtsIntegrationTeststasks — the 64 legacy JUnit4 tests and their suite tasks are untouched).:utskeeps three permanent, deep tier smoke tests (modeled on ably-cocoa#2223) that serve as infra acceptance and theuts/README.mdteaching examples.liveobjects/src/main) — op-path returnsObjectUpdate, plus two root-object safeguards.Based on
fix/liveobjects-objects-audit-op-handling(notmain).Related spec / cross-SDK context
LiveObjectUpdate; the internal-counter/map UTS tests assert the returned update directly.call_count == 1.FakeClockimplements the run-to-quiescence Guarantee and documents its advisory model (waitOnperforms a real timed wait — the root cause of a CI flake fixed here).Changes
1.
:uts= shared test-infra moduleuts/src/main/kotlin/io/ably/lib/uts/infra/(packages unchanged — zero import churn for consumers).:utsisjava-library+kotlin.jvm; deps:api(:java),api(:network-client-core), ktor asimplementation(never leaks), plus theapi-exported test toolkit. Declares Java-8 outgoing variants so:java(targetCompatibility 1.8) can consume it.uts/build.gradle.kts)::uts’s main configurations never depend on:liveobjects—:liveobjects test → :uts main → :javastays acyclic, and the oldtestRuntimeOnly(:liveobjects)edge is gone entirely (the objects tests now live inside:liveobjects).2. UTS suites in their owning modules
liveobjects/src/test/…/uts/{unit,integration,proxy}(packagesio.ably.lib.liveobjects.uts.*).:liveobjectsadopts the JUnit Platform (kotlin("test-junit5")pinned; vintage engine runs its own legacy JUnit4 tests).runLiveObjectsUnitTestsfilters both…unit.*and…uts.unit.*;runLiveObjectsIntegrationTestsadditionally covers…uts.{integration,proxy}.*.lib/src/test/kotlin/io/ably/lib/uts/…(:java; packages unchanged).:javagains the Kotlin plugin for tests only — a guardrail strips the auto-addedkotlin-stdlibfrom all main-artifact scopes (verified: POM and jar contain zero Kotlin; the leak source is the plugin, not the:utstest dependency).:utssmoke tests —UnitInfraSmokeTest(mock transport lifecycle incl. FakeClock cascades),IntegrationInfraSmokeTest(sandbox round-trip, protocol-parameterized),ProxyInfraSmokeTest(proxy session, declarative + imperative fault injection). Not spec-derived (no@UTSmarkers).check.ymlruns:java:runUtsUnitTests+:uts:runUtsUnitTestsalongside the existing tasks;integration-test.yml’scheck-utsruns:java:runUtsIntegrationTests+:uts:runUtsIntegrationTests;check-liveobjectspicks up the moved objects tiers via the extended filter.lib/src/test/kotlin/io/ably/lib/uts/deviations.md(realtime/rest) andliveobjects/…/uts/deviations.md(objects, all tiers).uts-to-kotlinskill’s mapping is simplified to one repo-root-relative path per tier; the resolver reports the owning Gradle module.3. Production spec-conformance (
liveobjects/src/main)ObjectUpdateinstead ofBoolean(RTLC9g/RTLM7f). The RTO9a2a4 on-ack serial gate now uses!update.noOp, matching the UTS model whereresult == true ⟺ !update.noop.LiveObjectTombstoneTest).4. Mock contract fixes (review-driven, verified against the UTS docs)
FakeClock.advanceruns due work to quiescence (cascades and timers created mid-advance fire within the same advance — spec Fixes in HttpScheduler.AsyncRequest #518’s Guarantee); timer state hardened against SDK-thread races. ThewaitOnadvisory seam is documented inuts/README.md§6.4, and the smoke test owns reconnect attempts deterministically (the CI-flake fix).cancel()deliverslistener.onClose(per the SDK’s ownWebSocketClientcontract);respondWithhonors theheadersparam and derives the body content-type case-insensitively;SandboxApp.create()checks HTTP status before parsing; teardown rethrowsCancellationException; assorted@Volatile/listener-cleanup hygiene.RTO24b1awaits its seed’s observable effect before subscribing (async-delivery race on slow runners), matching the file’s established pattern.Verification
:java:runUnitTests(legacy, count unchanged) ·:java:runUtsUnitTests6/0 ·:uts:runUtsUnitTests3/0 ·:liveobjects:runLiveObjectsUnitTests389/0.:java:runUtsIntegrationTests5/0 ·:uts:runUtsIntegrationTests4/0 ·:liveobjects:runLiveObjectsIntegrationTests29/0.:javaPOM/jar contain zeroorg.jetbrains.kotlinentries; jar file list byte-identical to pre-change.@UTStest-ID parity across all moves: identical sets before/after (zero coverage loss).checkWithCodenarc checkstyleMain checkstyleTest— green.Follow-up TODOs
Most of the original deferred list has since landed on this branch; what remains:
Extract shared infra so other modules consume it without— done, via promotion totestFixtures:uts’s main source set (chosen over a new:test-supportmodule: zero import churn, no settings change; rationale inFUTURE_WORK_UTS_INFRA.md).Kotlin test source set in— done (guardrail verified against POM/jar/runtimeClasspath).:javawith the kotlin-stdlib guardrailSimplify— done (resolver also reports the owning module).uts-package-mapping.jsonto one full path per tiertestImplementation(project(":uts"))) vs. separate-repo publish/version.:utsis publish-ready but publishing stays an explicit opt-in decision.ADVANCE_TIME(tracked alongside uts: define normative fake-time semantics (ADVANCE_TIME guarantee, hard-gate vs advisory, authoring rule) specification#518).Notes for reviewers
Read as four changes, not move noise: the op-path return and root-object safeguards are behavioural (§3); the mock contract fixes (§4) change test-infra behaviour; §1–§2 are structure. The 16 infra files and the realtime tests are pure renames (R100); the four moved objects tests changed only their
packagelines (plus one forced one-token smart-cast fix inAuthReauthTest, explained inline).uts/README.mdis rewritten around the new layout and its walkthroughs teach from the smoke tests;FUTURE_WORK_UTS_INFRA.mdis the decision record for how the final design was reached.